Skip to content

fix(material/chips): chips form control updating value immediately #30818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mistrykaran91
Copy link
Contributor

Currently, when we have chips with form control, the value is updated only when its focused out. This fix will update the value of form control immediately

Fixes #28065

@mistrykaran91 mistrykaran91 requested a review from a team as a code owner April 7, 2025 12:25
@mistrykaran91 mistrykaran91 requested review from crisbeto and wagnermaciel and removed request for a team April 7, 2025 12:25
@mistrykaran91
Copy link
Contributor Author

mistrykaran91 commented Apr 7, 2025

@mmalerba , I have raised a new PR here. The old one seems to be messed up while rebasing. Also, I have added here the extra check for disabled thingy. It might be because of this missing condition those internal google tests were failing ?

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Apr 15, 2025
@mmalerba
Copy link
Contributor

mmalerba commented Apr 15, 2025

CARETAKER NOTE: There are still 18 projects with failing tests. I think they're most likely just bad tests, where people expected the current behavior because it was the current behavior, rather than because it actually makes sense. But we will need to go through and fix those before we can land this

@mmalerba mmalerba added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: merge The PR is ready for merge by the caretaker labels Apr 15, 2025
@mmalerba
Copy link
Contributor

@mistrykaran91 I looked into one of the tests and the root of the problem seemed to be that a change event was fired even when the value didn't change. I think this is similar to what you were getting at with adding the disabled check, but it seems that's not the only scenario where it can erroneously emit a change.

@mistrykaran91
Copy link
Contributor Author

@mistrykaran91 I looked into one of the tests and the root of the problem seemed to be that a change event was fired even when the value didn't change. I think this is similar to what you were getting at with adding the disabled check, but it seems that's not the only scenario where it can erroneously emit a change.

Ok, Not sure how can I reproduce this one but i'll take a look and try to debug where it's failing

Currently, when we have chips with form control, the value is updated only when its focused out. This fix will update the value of form control immediately

Fixes angular#28065
@mistrykaran91 mistrykaran91 force-pushed the fix/chips-form-control-value-update-v2 branch from 9dff5e8 to 5915061 Compare April 22, 2025 04:39
@mistrykaran91 mistrykaran91 requested a review from mmalerba April 22, 2025 04:47
@mistrykaran91
Copy link
Contributor Author

@mmalerba : I have added one empty check before firing _change event, may be that should fix the issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: material/chips merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(MatChipGrid): Chips form controls do not update values until focus is achieved and lost
2 participants